feat(jira): add JIRA_SKIP_UNPARSEABLE_ISSUES to survive unparseable issue pages - #9026
feat(jira): add JIRA_SKIP_UNPARSEABLE_ISSUES to survive unparseable issue pages#9026vbhanuchander-lang wants to merge 2 commits into
Conversation
…ssue pages A page that comes back with a successful status but a body that is not the expected JSON - a truncated payload, or an HTML error page substituted by a proxy - fails the whole collectIssues subtask. On a large Jira instance that discards an otherwise complete sync because of a single bad page, typically during deep pagination with changelog expansion. Add JIRA_SKIP_UNPARSEABLE_ISSUES, read through the same taskCtx.GetConfigReader().GetBool() pattern that JIRA_JQL_AUTO_FULL_REFRESH already uses in this plugin. It defaults to false, preserving the current behaviour of surfacing the error and failing the subtask. When enabled, the offending page is logged with its URL and body size and skipped, and collection continues. Only parse failures on otherwise-successful responses are affected. Non-2xx statuses never reach the response parser and keep their existing retry behaviour. The V2 (api/2/search) and V3 (api/3/search/jql) response parsers were byte identical, so both now share a single parseIssuesResponse helper rather than carrying two copies of the change. A skipped page yields an empty non-nil slice, so the collector records a page with no rows instead of treating the result as absent. Signed-off-by: Bhanu Chander Vallabaneni <v.bhanuchander@gmail.com>
|
It makes sense to me. I wonder if the |
Requested in review on apache#9026. Signed-off-by: Bhanu Chander Vallabaneni <v.bhanuchander@gmail.com>
|
Thanks @klesh — I've pushed the On the status code: unfortunately it can't distinguish these, because the failing pages arrive with a successful status. That is also why the flag is deliberately narrow: HTTP error statuses keep their existing retry behaviour and are unaffected by it. |
2 participants
Summary
A Jira search page that comes back with a successful status but a body that is not the expected JSON — a truncated payload, or an HTML error page substituted by a proxy — currently fails the whole
collectIssuessubtask. On a large Jira instance that throws away an otherwise complete sync because of a single bad page, typically during deep pagination with changelog expansion.This adds
JIRA_SKIP_UNPARSEABLE_ISSUES:false— unchanged behaviour: the error surfaces and the subtask fails.true— the offending page is logged with its URL and body size, then skipped, and collection continues.The flag is read through the same
taskCtx.GetConfigReader().GetBool(...)pattern thatJIRA_JQL_AUTO_FULL_REFRESHalready uses in this plugin, and is logged once at the start of collection when enabled.Only parse failures on otherwise-successful responses are affected. Non-2xx statuses never reach the response parser and keep their existing retry behaviour, as the issue requested.
The V2 (
api/2/search) and V3 (api/3/search/jql) response parsers were byte-identical, so rather than duplicating the change they now share oneparseIssuesResponsehelper — a net reduction in duplicated code. A skipped page returns an empty non-nil slice, so the collector records "this page had no rows" instead of treating the result as absent.Does this close any open issues?
Closes #8949
Screenshots
N/A — behavioural change with no UI surface.
Other Information
Tests. Nine cases added to
issue_collector_test.go, following the existing table-driven style: valid pages under both flag states, the HTML-error-page and truncated-body variants under both flag states, a valid page with noissueskey, and the empty-vs-nil distinction on the skip path. Plus a smallresponseUrltest for nil-safety.One wrinkle worth flagging:
unithelper.DummyLogger()stubsWarnwith two arguments, but the real signature isWarn(err, format, a ...interface{}), which mockery records as three. The test adds the variadic stub locally rather than changing the shared helper or making the production call less idiomatic — variadiclogger.Warnis used widely across the plugins.Verification.
go test ./plugins/jira/...passes (plugins/jira/tasksok),gofmtclean,go vetclean for the changed files. Across the repo's unit gate, the only failures I see locally are thegitextractorpackages failing to build becausepkg-config/libgit2 isn't installed on my machine — unrelated to this change.e2eis excluded from the unit gate and needsE2E_DB_URL.Documentation. I have not added docs here, matching the existing precedent:
JIRA_JQL_AUTO_FULL_REFRESHand the other plugin-level flags are not documented in this repo, andenv.examplecarries only global settings. Happy to send a companion PR to the website repo if that is the preferred home for it.Scope question for maintainers. I have implemented this in the Jira plugin as the issue asks, but the same "2xx with an unparseable body" failure can affect any collector. If you would rather see it handled generically in the shared collector helper, I am glad to rework it.
Coordination. @Murad-Suleymanov opened the issue and mentioned they were willing to submit a PR. I commented on the issue before opening this and will happily close it if they already have work in progress.